feat: add slient option #1712

pkafei · 2018-11-15T09:35:23Z

This PR resolves #1683. This PR is not merge-ready and I wanted to highlight some of the issues I've currently ran into.

node src/cli/bin daemon --silent does not disable printing "Swarming listening on..." and I'm not sure exactly why this option is not picked up by the core library.

Also, when I use print as a property the peer address is printed on a new line. Not sure how to remedy this behavior but I did try using process.stdout.write(). https://stackoverflow.com/questions/6157497/node-js-printing-to-console-without-a-trailing-newline

However when I implemented process.stdout.write() the address came before the print statement process.stdout.write()

Initializing IPFS daemon...
/ip4/127.0.0.1/tcp/4003/ws/ipfs/QmYT3Sv3paCcitjsSbBaeCuXRjcpTs4J87hVPyS4YLiji6Swarm listening on
/ip4/127.0.0.1/tcp/4002/ipfs/QmYT3Sv3paCcitjsSbBaeCuXRjcpTs4J87hVPyS4YLiji6Swarm listening on
/ip4/192.168.1.27/tcp/4002/ipfs/QmYT3Sv3paCcitjsSbBaeCuXRjcpTs4J87hVPyS4YLiji6Swarm listening on
/ip4/10.52.0.22/tcp/4002/ipfs/QmYT3Sv3paCcitjsSbBaeCuXRjcpTs4J87hVPyS4YLiji6Swarm listening on
API listening on /ip4/127.0.0.1/tcp/5002
Gateway (read only) listening on /ip4/127.0.0.1/tcp/9090
Web UI available at http://127.0.0.1:5002/webui

hugomrdias · 2018-11-15T10:18:52Z

hi @pkafei CI lint jobs are failing check https://github.com/ipfs/community/blob/master/CONTRIBUTING_JS.md#commits for commit guidelines and also run npm run lint to check eslint errors.

pkafei · 2018-11-15T14:14:19Z

@hugomrdias I don't understand the output "Failed to extract deps-macos-10.11.0.tar.gz"- this doesn't seem like it's a JS style error. https://ci.ipfs.team/blue/organizations/jenkins/IPFS%2Fjs-ipfs/detail/ipfs-cli-fun/1/pipeline

achingbrain · 2018-11-15T17:42:26Z

Failed to extract deps-macos-10.11.0.tar.gz

This is Jenkins running out of disk space on the worker in question. You can tell by opening the full console output for the build and looking for log lines that ran on that worker (e.g. search the output for macos:10.11.0 test:node).

@hugomrdias is right though, the commit lint failed the build in an earlier step though neglected to tell anyone. Thanks Jenkins.

pkafei · 2018-11-15T17:50:14Z

@achingbrain I suspected this might be a build error. So what are the next steps? Should I ignore the error message or is there something I can do to make this test pass?

hugomrdias · 2018-11-15T18:00:34Z

@pkafei first fix code lint errors and commit lint errors

pkafei · 2018-11-15T18:03:26Z

@hugomrdias @achingbrain Wait, I'm confused- is the lint error based on a JS style issue or is it a build problem. If it is the latter, I'm not sure how to go about fixing the build issue.

hugomrdias · 2018-11-15T18:07:45Z

Just ignore this error Failed to extract deps-macos-10.11.0.tar.gz

your commit looks like First attempt at activating silent option in cli and that doesn't comply with our guidelines https://github.com/ipfs/community/blob/master/CONTRIBUTING_JS.md#commits

also in your computer run npm run lint and you will see errors, please fix these and push again.

alanshaw · 2018-11-16T10:55:42Z

src/cli/utils.js

@@ -47,6 +47,7 @@ exports.getIPFS = (argv, callback) => {
  // Required inline to reduce startup time
  const IPFS = require('../core')
  const node = new IPFS({
+    print: argv.print,


Suggested change

print: argv.print,

silent: argv.silent,

The option name is silent, not print.

alanshaw · 2018-11-16T10:56:03Z

src/core/components/libp2p.js

@@ -96,7 +96,7 @@ module.exports = function libp2p (self) {
          if (err) { return callback(err) }

          self._libp2pNode.peerInfo.multiaddrs.forEach((ma) => {
-            console.log('Swarm listening on', ma.toString())
+            self._print('Swarm listening on ', console.log(ma.toString()))


Suggested change

self._print('Swarm listening on ', console.log(ma.toString()))

self._print('Swarm listening on', ma.toString())

alanshaw · 2018-11-16T11:02:09Z

src/core/index.js

@@ -108,6 +108,8 @@ class IPFS extends EventEmitter {
    this._preload = preload(this)
    this._mfsPreload = mfsPreload(this)
    this._ipns = new IPNS(null, this)
+    this._print = options.silent ? (() => {}) : ((msg) => console.log(msg))


I think it might be a good idea to move printed messages into the debug log if silent=true, so if you specify DEBUG=jsipfs* jsipfs daemon --silent you'll still be able to access that information - what do you think?

Suggested change

this._print = options.silent ? (() => {}) : ((msg) => console.log(msg))

this._print = this._options.silent ? this.log : console.log

Also, we should pass the console.log funciton directly here so that we can log multiple args.

When I pass the --silent as an option in the command line I'm still receiving Swarm addresses as output in the terminal.

Try logging out the value of this._options.silent here and argv.silent in src/cli/utils.js . Maybe it's not being passed in correctly?

Did you figure this out in the end?

Haven't had a chance to work on this in the past couple of days, but plan on getting back to this this afternoon.

alanshaw · 2018-11-29T09:02:56Z

Would you mind rebasing for green CI please?

alanshaw

Code LGTM, next steps for this PR are to add a test and document the new option on the README here.

achingbrain · 2018-12-03T11:57:31Z

package.json

@@ -167,12 +167,13 @@
    "read-pkg-up": "^4.0.0",
    "readable-stream": "3.0.6",
    "receptacle": "^1.3.2",
+    "sinon": "^7.1.1",


This should be in the devDependencies block..

alanshaw

Looking good to me, just a couple of tweaks to the documentation and I think we'll be good for merge.

alanshaw · 2018-12-04T08:46:16Z

README.md

+
+| Type | Default |
+|------|---------|
+| string | `null` |


Type is Boolean and default is false

alanshaw · 2018-12-04T08:48:49Z

README.md

+|------|---------|
+| string | `null` |
+
+The silent option prevents swarm info output in the cli and http client.


"Prevents all logging output from the IPFS node"

License: MIT Signed-off-by: Alan Shaw <alan.shaw@protocol.ai>

alanshaw · 2018-12-07T16:45:16Z

@pkafei I took a quick look and resolved the issue with the libp2p tests. Hopefully CI will be 🍏 now.

I've rebased the branch again. 🚢 🚢 🚢

pkafei · 2018-12-07T17:02:51Z

@alanshaw That's exciting news! It won't be long

pkafei added the status/in-progress In progress label Nov 15, 2018

pkafei self-assigned this Nov 15, 2018

pkafei requested a review from alanshaw November 15, 2018 09:35

alanshaw suggested changes Nov 16, 2018

View reviewed changes

alanshaw suggested changes Nov 29, 2018

View reviewed changes

alanshaw mentioned this pull request Nov 29, 2018

Turn off logger? #1683

Closed

achingbrain reviewed Dec 3, 2018

View reviewed changes

alanshaw suggested changes Dec 4, 2018

View reviewed changes

alanshaw changed the title ~~First attempt at activating silent option in cli~~ feat: add slient option Dec 4, 2018

pkafei and others added 9 commits December 7, 2018 16:12

First attempt at activating silent option in cli

22ed138

Updated Changes

ec4b3e3

Swarm output is silenced

f3b9aae

All messages are silinced

51f1999

fix: tests for silent option

75d6b01

fix: added test for silent argument in cli api

563baf3

fix: added docs

84b37ed

fix: small edits to documentation

f978496

fix: libp2p tests

d075bdc

License: MIT Signed-off-by: Alan Shaw <alan.shaw@protocol.ai>

alanshaw force-pushed the logging-bug branch from b2c784d to d075bdc Compare December 7, 2018 16:43

ghost assigned alanshaw Dec 7, 2018

alanshaw approved these changes Dec 7, 2018

View reviewed changes

alanshaw merged commit 593334b into master Dec 7, 2018

ghost removed the status/in-progress In progress label Dec 7, 2018

alanshaw deleted the logging-bug branch December 7, 2018 18:14

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: add slient option #1712

feat: add slient option #1712

pkafei commented Nov 15, 2018 •

edited by alanshaw

Loading

hugomrdias commented Nov 15, 2018

pkafei commented Nov 15, 2018

achingbrain commented Nov 15, 2018 •

edited

Loading

pkafei commented Nov 15, 2018

hugomrdias commented Nov 15, 2018

pkafei commented Nov 15, 2018

hugomrdias commented Nov 15, 2018

alanshaw Nov 16, 2018

alanshaw Nov 16, 2018

alanshaw Nov 16, 2018

alanshaw Nov 16, 2018

pkafei Nov 16, 2018

alanshaw Nov 16, 2018

alanshaw Nov 20, 2018

pkafei Nov 20, 2018

alanshaw commented Nov 29, 2018

alanshaw left a comment

achingbrain Dec 3, 2018

alanshaw left a comment

alanshaw Dec 4, 2018

alanshaw Dec 4, 2018

alanshaw commented Dec 7, 2018

pkafei commented Dec 7, 2018

	self._print('Swarm listening on ', console.log(ma.toString()))
	self._print('Swarm listening on', ma.toString())

	this._print = options.silent ? (() => {}) : ((msg) => console.log(msg))
	this._print = this._options.silent ? this.log : console.log

feat: add slient option #1712

feat: add slient option #1712

Conversation

pkafei commented Nov 15, 2018 • edited by alanshaw Loading

hugomrdias commented Nov 15, 2018

pkafei commented Nov 15, 2018

achingbrain commented Nov 15, 2018 • edited Loading

pkafei commented Nov 15, 2018

hugomrdias commented Nov 15, 2018

pkafei commented Nov 15, 2018

hugomrdias commented Nov 15, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

alanshaw commented Nov 29, 2018

alanshaw left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

alanshaw left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

alanshaw commented Dec 7, 2018

pkafei commented Dec 7, 2018

pkafei commented Nov 15, 2018 •

edited by alanshaw

Loading

achingbrain commented Nov 15, 2018 •

edited

Loading